-
Notifications
You must be signed in to change notification settings - Fork 12k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make __libcpp_verbose_abort() noexcept like std::terminate() #109151
Conversation
@llvm/pr-subscribers-libcxx Author: Doug Wyatt (dougsonos) ChangesMake __libcpp_verbose_abort() noexcept (it is already noreturn), to match std::terminate(). Clang's function effect analysis can use this to ignore such functions as being beyond its scope. (See #99656) Full diff: https://github.com/llvm/llvm-project/pull/109151.diff 1 Files Affected:
diff --git a/libcxx/include/__verbose_abort b/libcxx/include/__verbose_abort
index 244278aec652d2..45a0add493c17d 100644
--- a/libcxx/include/__verbose_abort
+++ b/libcxx/include/__verbose_abort
@@ -21,7 +21,8 @@ _LIBCPP_BEGIN_NAMESPACE_STD
// This function should never be called directly from the code -- it should only be called through
// the _LIBCPP_VERBOSE_ABORT macro.
[[__noreturn__]] _LIBCPP_AVAILABILITY_VERBOSE_ABORT _LIBCPP_OVERRIDABLE_FUNC_VIS
-_LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2) void __libcpp_verbose_abort(const char* __format, ...);
+_LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2) void __libcpp_verbose_abort(const char* __format, ...)
+_NOEXCEPT;
// _LIBCPP_VERBOSE_ABORT(format, args...)
//
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Oops, I forgot to make the implementation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me, however this is technically a breaking change because in the past we have advertised __libcpp_verbose_abort
as being overridable (that's why it was a weak def).
We'd also want to add a release note for this change in libcxx/docs/ReleaseNotes/20.rst
, probably under Deprecations and Removals
.
Pinging @llvm/libcxx-vendors for awareness.
Any pointers on what's going on with the test builds? I click Details and just see "job failed". |
Our CI is currently in a really bad state. Workers are failing to connect. This has been going on for a few days and we're trying to resolve the issue. Sorry for the inconvenience. |
Please rebase onto main to re-trigger CI. The CI instability should be resolved now. |
e8d6586
to
b99a598
Compare
match std::terminate(). Clang's function effect analysis can use this to ignore such functions as being beyond its scope.
b99a598
to
afbbb98
Compare
I should have trusted the CI and seen this error!
This is in a C++03 test. |
Ping |
@dougsonos this breaks flang compilation because flang overrides this symbol and doesn't use |
Apologies. Can we fix flang to match? |
In fact, we want to move away from supporting overriding that function. The way of customizing the assertion handler is when configuring the library, as explained here: https://libcxx.llvm.org/Hardening.html#overriding-the-assertion-failure-handler |
@ldionne - The issue here from the Flang perspective, is not that flang want to override the function, they don't really care. The thing is that Flang uses some classes from the C++ standard library (from |
Moving the discussion to #113059 (comment) to centralize things. |
Hi @dougsonos Unfortunately this is breaking when trying to build Fuchsia with a clang version that includes this change. Here is a link to the build failure we're seeing: There are places in the Fuchsia checkout that provide their own definitions of this function: https://cs.opensource.google/search?q=__libcpp_verbose_abort Can you make this backwards compatible to enable us to transition to the |
…llvm#109151) Make __libcpp_verbose_abort() noexcept (it is already noreturn), to match std::terminate(). Clang's function effect analysis can use this to ignore such functions as being beyond its scope. (See llvm#99656).
…xcept again (llvm#113310) This allows a slightly smoother transition for people after llvm#109151, as requested on that PR.
Make __libcpp_verbose_abort() noexcept (it is already noreturn), to match std::terminate(). Clang's function effect analysis can use this to ignore such functions as being beyond its scope. (See #99656)